Fix subtype checking for nested captured types by msridhar · Pull Request #1663 · uber/NullAway

msridhar · 2026-07-28T14:20:12Z

The new test wildcardCaptureReturnPreservesNestedNullability is the most direct test for this gap. The key logic fix is in CheckIdenticalNullabilityVisitor#visitClassType, where we use the effective upper bound of rhsType when it is a wildcard or captured type. The change is otherwise a bit larger than expected to ensure checking doesn't break when the config setting to handle wildcards is disabled. We also extract the haveIdenticalNullability function to de-duplicate some logic.

Summary by CodeRabbit

Bug Fixes
- Improved nullability checking for wildcard generic types.
- Prevented incorrect compatibility results and crashes involving wildcard captures, nested nullable types, and method references.
- Ensured wildcard handling respects the configured settings.
Tests
- Added regression coverage for wildcard returns, method-reference parameters, and unbounded wildcard scenarios.
Documentation
- Clarified that JUnit test methods do not require Javadoc.

msridhar · 2026-07-28T14:20:13Z

This change is part of the following stack:

Fix subtype checking for nested captured types #1663 ◀
- Handle interaction of captured types and library models #1666
  - Follow-on fix for restoring annotation on captured type wildcards #1667
    - Harden TypeSubstitutionUtils type copies #1669
      - Handle unbound wildcards and interactions with captured types better #1668
        
        Improve error messages involving capture variables #1675

_{Change managed by git-spice.}

coderabbitai · 2026-07-28T14:23:04Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

CheckIdenticalNullabilityVisitor gates wildcard handling on configuration, resolves captured RHS types to effective wildcard upper bounds, and delegates non-wildcard comparisons to haveIdenticalNullability. WildcardTests adds nested wildcard and method-reference regression cases. AGENTS.md clarifies that JUnit test methods do not require Javadoc.

Possibly related PRs

uber/NullAway#1520: Modifies the same visitor's wildcard handling and related tests.
uber/NullAway#1662: Covers related wildcard detection and containment behavior.
uber/NullAway#1577: Updates overlapping wildcard nullability comparison logic.

Suggested reviewers: lazaroclapp

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing subtype checking for nested captured types.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch nested-captures-restored-ananotations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-28T14:30:06Z

Codecov Report

❌ Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.70%. Comparing base (b282e16) to head (ec8cb15).

Files with missing lines	Patch %	Lines
...way/generics/CheckIdenticalNullabilityVisitor.java	79.16%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1663      +/-   ##
============================================
+ Coverage     87.67%   87.70%   +0.02%     
- Complexity     3132     3136       +4     
============================================
  Files           109      109              
  Lines         10580    10586       +6     
  Branches       2136     2140       +4     
============================================
+ Hits           9276     9284       +8     
+ Misses          627      625       -2     
  Partials        677      677

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java`:
- Around line 41-60: Add Javadoc to the non-trivial `visitClassType` method in
`CheckIdenticalNullabilityVisitor`, documenting its purpose and the relevant
wildcard/captured-type handling, including the behavior controlled by
`handleWildcardGenerics()`. Keep the implementation unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b24f7c50-1799-4ca5-a9f0-5d04538ce4c2

📥 Commits

Reviewing files that changed from the base of the PR and between 08a25a9 and 272c957.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java
nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java`:
- Line 124: Add Javadoc to the non-trivial visitArrayType method, documenting
its purpose, parameters, and return value while accurately describing how it
compares array component nullability. Do not alter the existing
haveIdenticalNullability behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aec8c925-8f44-48a2-b812-37d0eb7717c6

📥 Commits

Reviewing files that changed from the base of the PR and between 272c957 and 5157eb1.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java
nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

lazaroclapp

LGTM!

lazaroclapp · 2026-08-05T01:54:09Z

        .doTest();
  }

+  /** ensures we avoid a crash related to wildcards when wildcard handling is disabled */


What is the behavior when we do have wildcard handling enabled? Is that handled on a separate PR on the chain?

Wildcard handling is enabled by default for our regression tests now. It's included as part of the JSpecify experimental flag that we include for any JSpecify test, which also includes the JSpecify JDK library models

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java

msridhar requested a review from lazaroclapp July 28, 2026 17:28

Base automatically changed from captured-type-wildcard-containment to master July 29, 2026 16:14

msridhar force-pushed the nested-captures-restored-ananotations branch 2 times, most recently from 9583b50 to f54e916 Compare July 30, 2026 01:17

msridhar force-pushed the nested-captures-restored-ananotations branch from f54e916 to 61c62c6 Compare August 2, 2026 02:09

msridhar added 6 commits August 4, 2026 13:36

Handle captured types when doing substitutions

4a94a89

Fix top-level bug

3838064

remove some changes

0e13d56

fix for outside experimental mode

7b3de10

more

9669ea8

improve tests

ec8cb15

msridhar force-pushed the nested-captures-restored-ananotations branch from 61c62c6 to ec8cb15 Compare August 4, 2026 20:37

msridhar mentioned this pull request Aug 5, 2026

Improve error messages involving capture variables #1675

Open

lazaroclapp approved these changes Aug 5, 2026

View reviewed changes

msridhar merged commit a47afb5 into master Aug 5, 2026
13 of 14 checks passed

msridhar deleted the nested-captures-restored-ananotations branch August 5, 2026 02:05

Navigation Menu

Uh oh!

Fix subtype checking for nested captured types - #1663

Fix subtype checking for nested captured types#1663
msridhar merged 6 commits into
masterfrom
nested-captures-restored-ananotations

msridhar commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

msridhar commented Jul 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Reviews paused

Uh oh!

codecov Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

lazaroclapp left a comment

Uh oh!

lazaroclapp Aug 5, 2026

Uh oh!

msridhar Aug 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

msridhar commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

msridhar commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Uh oh!

codecov Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lazaroclapp left a comment

Choose a reason for hiding this comment

Uh oh!

lazaroclapp Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

msridhar Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

msridhar commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

msridhar commented Jul 28, 2026 •

edited

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

codecov Bot commented Jul 28, 2026 •

edited

Loading